Skip to content

enable scrolling inside of iframes #919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 13, 2025

Conversation

seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Jul 26, 2025

why

this PR addresses two issues, both related to scrolling inside iframes:

  1. we are not calling .evaluate inside frames:
    • currently, the scrolling functionality that exists inside performPlaywrightMethod calls page.evaluate
    • this means that scrolling can only ever happen at the page level, not inside of iframes
    • inside each of these scroll related helpers, we already have access to a chained locator that optionally points to an element inside an iframe
    • therefore, we should use this chained locator, and call locator.evaluate
  2. for SPIFs (same process iframes), we are not looking for scrollable elements in the correct execution context:
    • when we call resolveObjectIdForXPath, this executes in either the page level execution context, or, for OOPIFs (out of process iframes), the frame level execution context
    • this is problematic because SPIFs share the the same CDP session as the root document, which means that we are responsible for specifying the execution context. since we aren't doing this, resolveObjectIdForXPath only searches in the root document execution context, and can't see anything inside of the SPIF

what changed

  • to address issue number 1, I updated scrollToNextChunk, scrollToPreviousChunk, scrollElementToPercentage all use locator.evaluate instead of page.evaluate which enables scrolling inside (and outside) of iframes
  • to address issue number 2, I added a function getFrameExecutionContextId which creates an isolated world & returns a SPIF scoped execution context
    • we use this downstream in resolveObjectIdForXPath which guarantees that we are searching for scrollable elements in the correct execution context

test plan

  • added an eval for scrolling inside a same-process iframe
  • act evals
  • targeted_extract evals
  • observe evals
  • extract evals

Copy link

changeset-bot bot commented Jul 26, 2025

🦋 Changeset detected

Latest commit: 972a190

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@seanmcguire12 seanmcguire12 force-pushed the sean/stg-609-enable-scrolling-inside-of-iframes branch from 0d329a8 to ba001ca Compare August 12, 2025 19:58
@seanmcguire12 seanmcguire12 marked this pull request as ready for review August 13, 2025 00:02
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR fixes a critical limitation in Stagehand where scrolling actions could not be performed inside iframes. The changes address two specific technical issues:

  1. Execution Context Problem: The original implementation used page.evaluate for all scrolling operations, which meant scrolling could only occur at the page level and not within iframe contexts. The fix refactors three key scrolling functions (scrollToNextChunk, scrollToPreviousChunk, and scrollElementToPercentage) in actHandlerUtils.ts to use locator.evaluate instead, leveraging the locator's inherent iframe-awareness.

  2. Same Process Iframe (SPIF) Context Resolution: For SPIFs that share the same Chrome DevTools Protocol (CDP) session as the root document, the code wasn't properly specifying execution contexts when resolving XPaths for scrollable elements. This caused resolveObjectIdForXPath to only search in the root document context, missing elements inside SPIFs. The solution introduces a new getFrameExecutionContextId function in utils.ts that creates isolated worlds for SPIFs, ensuring XPath evaluation happens in the correct execution context.

The changes integrate seamlessly with Stagehand's existing iframe handling architecture by building upon Playwright's locator system rather than introducing new iframe traversal logic. A new evaluation test iframe_scroll has been added to validate the functionality, testing that the act method can successfully scroll within a same-process iframe and verify the scroll position.

Confidence score: 3/5

  • This PR addresses real functionality gaps but has several implementation concerns that could cause issues
  • Score reflects solid architectural improvements but potential reliability issues in the test implementation and some complex CDP context handling
  • Pay close attention to evals/tasks/iframe_scroll.ts for brittle frame access patterns and lib/a11y/utils.ts for CDP context management complexity

4 files reviewed, 3 comments

Edit Code Review Bot Settings | Greptile

@seanmcguire12 seanmcguire12 added act These changes pertain to the act function extract These changes pertain to the extract function observe These changes pertain to the observe function targeted-extract These changes pertain to targeted extract labels Aug 13, 2025
@seanmcguire12 seanmcguire12 removed act These changes pertain to the act function extract These changes pertain to the extract function observe These changes pertain to the observe function targeted-extract These changes pertain to targeted extract labels Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants